feat: worker liveness verification + orphaned-agent recovery (D7)#60
Merged
Conversation
Loopy consumer side of team-harness TH-D5 (proposals P0.1 liveness slice +
P2.5), per decision D7's ownership split.
Worker liveness (makes reclaim safe, not optimistic):
- The worker sends its process identity (hostname + pid + team-harness's
pid-reuse-proof starttime token) with /register and /finished; the
coordinator stamps it onto every dispatched CurrentTask (register,
finished, child dispatch, parent resume).
- A /register while the recorded worker is VERIFIABLY alive returns HTTP
409 instead of abandoning live work — closing the duplicate-work window.
Verification is same-host + token match; unknown identities (old workers,
remote hosts, no token) preserve the pre-existing behavior. The wire
change is backward compatible ({} register still works).
- The bundled worker exits with code 3 on 409 and uses an unbounded read
timeout (recovery can legitimately block /register up to the drain
deadline).
Orphaned-agent recovery (bounded drain by default):
- On a confirmed-dead worker with nothing recoverable, the coordinator
discovers the interrupted harness run(s) from the iteration's
harness_outputs directory and applies recovery_policy (new config:
drain|reap, default drain bounded by recovery_drain_timeout_s, default
600s) via team-harness's reaper. Drained agents finish; their repo edits
survive (D1); the iteration is still re-run and its result is never
fabricated (D3).
- salvage.json in the interrupted iteration directory records the reap
reports; the history entry is abandoned_after_drain instead of abandoned,
so "crashed but salvaged" and "crashed, lost" are distinguishable.
- team-harness's parent-liveness guard (ReapRefusedError) surfaces as 409.
- team-harness is resolved at call time (importlib): versions without the
reaper (<= 0.2.10) degrade gracefully to plain abandonment.
New modules: worker_identity.py (capture + verified liveness),
recovery.py (discovery + reap + salvage record). Config: recovery_policy,
recovery_drain_timeout_s (also in the snapshot). Docs: http-contract,
session-layout, README, website MDX pages (http-contract, session-layout,
configuration), CHANGELOG.
Tests: 19 new (135 total) — verified-alive 409 (state untouched),
verified-dead + unknown-identity recovery, abandoned_after_drain,
RecoveryRefused 409, identity stamping via register and finished, empty-body
compatibility, recovery module (fake reaper: discovery, salvage schema,
refusal, degraded mode, no-runs no-salvage), worker payload identity, 409
exit code, config defaults/overrides. Integration-verified against the real
team-harness feat/th-d5-process-lifecycle branch in a separate venv: a real
orphan was drained through the real reaper (salvage.json written, run.json
status drained); the pinned 0.2.10 suite run verifies the degraded path.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt
Both reviewers (Codex gpt-5.6-sol xhigh, Antigravity) returned request-changes on c6316cc; this addresses their reconciled findings. Protocol compatibility (Codex M7 — the compat claim was false): - recovery_policy / recovery_drain_timeout_s removed from the wire RootConfigSnapshot: released workers validate the snapshot with extra="forbid", so the new response fields would have crashed every old worker at parse time. The settings are coordinator-side only and excluded when snapshots are built. Liveness correctness (both reviews' top finding + Codex M6): - is_worker_alive distinguishes "pid gone" from "cannot capture": a coordinator whose team-harness lacks process identity (version skew) now returns unknown -> legacy behavior, never "verified dead" -> duplicate dispatch. Zombies count as dead (an exited-but-unreaped worker cannot hold its task hostage via 409). - The unbounded read timeout applies to /register ONLY; /finished keeps the bounded default so a wedged response cannot leave a worker alive-but-stuck forever. 409 messages name the pid and the kill-and-reregister escape hatch for hung workers. Recovery safety (Codex M2/M3/M5/m1, Antigravity 3/4/5): - Two-phase /register: the potentially long drain runs OUTSIDE the state lock (phase A), with a locked re-validating commit (phase B) that replans when state moved in between — loopy status/stop and /finished stay usable during recovery; lock contention surfaces as HTTP 503 and friendly CLI errors instead of raw tracebacks. - Unsettled outcomes block dispatch: if any orphan may still be running after recovery (unverifiable identity, probe failure, kill that did not land), /register returns 409 instead of starting a second writer; the salvage record (now with unsettled_workers, written even when a mid-loop refusal aborts recovery) names the unresolved processes. - Same-host gate: a worker identity from another hostname skips local reaping (signals cannot reach it; shared-filesystem deployments would otherwise probe the wrong host's pids and report false success). - Run-record validation: a discovered directory name is only reaped when its run.json's run_id and session_output_dir point back at this iteration (stray names cannot select an unrelated run). - ONE recovery_drain_timeout_s deadline shared across all of the iteration's runs (was per-run). Ownership (Codex M8): - A stale /finished from a DIFFERENT identified worker is refused (409) instead of being handed the live task (second executor); unknown identities keep the pre-existing stale-retry behavior. Audit honesty (Codex m2, Antigravity 7): - History error is abandoned_after_<policy> (drain/reap), only when something actually settled. Docs: http-contract (canonical + website), session-layout (both), configuration, README, CHANGELOG all realigned with the implemented behavior (same-host-only recovery, hung-worker escape hatch, shared deadline, coordinator-only settings, stale-finished 409, 503 semantics). Tests: 6 added (141 total) — version-skew unknown, zombie-as-dead, stray-directory skip, unsettled -> RecoveryIncompleteError with salvage, stale-finished 409 + legacy passthrough, no-recovery-fields-on-the-wire. Integration re-verified against the real team-harness branch: a real orphan drained through the hardened flow (record-match guard passing, salvage with settled/unsettled counts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt
… owner The maintainer confirmed backward compatibility is not required, which unlocks closing the one residual duplicate-execution window the compat compromise had left open (Codex M8's weak branch): - /register without a worker identity is rejected with HTTP 400 (breaking; pre-0.3 workers cannot register — upgrade workers and coordinator together). Every dispatched task therefore has a recorded owner and liveness verification is always at least attemptable. - A stale /finished is replayed ONLY to the task's recorded owner; any other caller — identified or anonymous — gets HTTP 409. A task persisted by a pre-identity version (owner None) keeps the legacy replay for that one resume. Deliberately KEPT despite permission to break: - recovery settings stay off the wire snapshot (coordinator-only concern — better design regardless of compat); - the "unknown" liveness verdict and importlib soft-resolution remain: they guard against runtime environment facts (remote hosts, a team-harness without process identity), not old clients. Docs (http-contract both copies, CHANGELOG breaking header per the 0.2.0 house style) and tests updated: register-requires-identity, anonymous stale 409, owner replay, pre-upgrade ownerless replay. 143 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt
21e4216 to
8277b2d
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The loopy-loop consumer side of team-harness TH-D5 (proposals P0.1 worker-liveness slice + P2.5), per decision D7's ownership split:
{hostname, pid, starttime-token}with/registerand/finished; a register without identity is rejected with HTTP 400 (pre-0.3 workers can't register — upgrade workers and coordinator together). Every dispatched task therefore has a recorded owner./registerwhile the recorded worker is verifiably alive (same host, token match, not a zombie) returns HTTP 409 instead of abandoning live work. Unverifiable (remote host, old team-harness) → pre-existing recovery behavior./finishedis replayed only to the task's recorded owner — any other caller, identified or anonymous, gets 409. This closes the last duplicate-execution window (the compat compromise Codex flagged); a pre-identity persisted task keeps the legacy replay for that one resume.recovery_policy(coordinator-side config; default bounded drain, one shared deadline) via team-harness's reaper — outside the state lock (loopy status/stopstay usable).salvage.json+abandoned_after_<policy>history; unsettled orphans → 409, never a second writer.Review process
Adversarially reviewed by Codex (gpt-5.6-sol @ xhigh) and Antigravity — both request-changes; all reconciled findings addressed. Highlights:
extra="forbid") → recovery settings are coordinator-only, never on the wire (kept even after compat was waived — it's better design)./finishedkeeps its bounded timeout, zombie-as-dead, policy-honest audit labels, 503 on lock contention./finishedhanded the live task to a different worker → now owner-only (fully closed by the breaking change; the maintainer confirmed backward compatibility is not required).Verification
team-harness@feat/th-d5-process-lifecycle): identity verified alive/dead via real tokens; a real orphan drained through the hardened flow — record-match guard passing, salvage with settled/unsettled counts.Dependencies / notes
/registerrequires identity (CHANGELOG carries the migration note, per the 0.2.0 house style).docs/http-contract.md: same-host-only recovery; a hung-but-alive worker requires killing the process (the 409 names its pid);/registercan block ~drain-deadline (bundled worker uses an unbounded read timeout on register only).🤖 Generated with Claude Code
https://claude.ai/code/session_01PN8aFwwxA8FzMy9LgpbQFt